Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@mukeshpanchal27 Heads up that the tests in #7344 have been updated to reflect the last few comments there.

Nice one @mukeshpanchal27. @dlh01 and @joemcgill, it looks like all the unit tests we could think of pass in this branch. How do you feel about the proposed solution? Is it good enough to move forward as part of the WP 6.7 release?

Thanks for the ping! I should be able to take a look at this Wednesday or Thursday.

The PR is ready for review. Address the feedbacks.

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props mukesh27, flixos90, gziolo, dlh, joemcgill, santosguillamot.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Yes, that’s exactly what I was thinking about in terms of backward compatibility. It seems like a viable path forward. We can explore specific details of the depreciation strategy separately later. I was thinking also about making the context a virtual property and handle everything through magic methods. Whatever works best here 😀

I was thinking also about making the context a virtual property and handle everything through magic methods.

That was my first thought too, but I think that wouldn't work because magic methods would only be called for the entire $context property, so we wouldn't know which keys someone is accessing on it. That's how I got to the ArrayAccess class idea.

I think got it know, we would pass WP_Block_Context instance to the filter when processing the inner block. This way we would be able to easily derive the context update, based on detected changes in the class. Interesting idea 👍🏻

I think got it know, we would pass WP_Block_Context instance to the filter when processing the inner block. This way we would be able to easily derive the context update, based on detected changes in the class. Interesting idea 👍🏻

That's actually not entirely what I thought, but that's even better than what I thought 😆 So let's try that.

@gziolo and @felixarntz, I've been spending time today catching up on this conversation and wanted to summarize what I understand to be the current thinking. Based on what @gziolo said is his preferred solution, the render_block_context filter should be filtering the available_context that a block can access, based on it's own uses_context properties and NOT be filtering the actual context property of a block before it is rendered.

Conceptually, this makes sense to me, and I think could be achieved by filtering the $child_context passed to WP_Block_List here.

In practice, however, the render_block_context filter seems to be used to affect the context property during rendering even in Core's own application of the filter in the comment template block and the post template block, so I do think planning a deprecation path as has been discussed would be important and would also likely require additional changes to the places where context is being dynamically filtered in certain blocks.

I've put together the following visual to help illustrate the concept of block context as it currently exists:

Restating what is in the illustration:

1. Initially, the available context gets filtered before the root block is initialized (ref)
2. The root block adds additional context to the available context prior to instantiating innerBlocks (ref)
3. innerBlock context gets filtered during rendering, but available context is not affected by this filter (ref)

If applying the filter to available_context is the goal, I think we should start with fixing that bug, as is the expected behavior preferred by @dlh01 in the original ticket description:

Between the two options, I would expect the filter to behave like it does in render_block(): Context supplied with a filter is made available to inner blocks.

Whether or not we then change the current ability for a block's context property to be modified to include values that a block itself does not have listed in its 'uses_context' property can be a separate decision.

Thanks for the helpful summary and visualization @joemcgill.

It makes sense for now to focus on only addressing the bug that $available_context of an inner block is currently not affected and thus not correctly passed to its child blocks.

As far as I can tell, to fix that this PR is almost there. It would be great to get your code review as well based on your understanding.

@joemcgill, thank you for your analysis to this complex problem. I agree with the proposed path forward, where the focus moves on solving one problem at a time. In effect, we seem to have a path forward to land this PR after addressing the feedback continued by @felixarntz. I'm happy to have another look from that angle next week.

@felixarntz I have addressed the recent feedback and PR is ready for review.

I'm happy to have another look from that angle next week.

@gziolo Happy to get your feedback if you have moments. Thanks.

Thanks, @felixarntz, for the feedback! The unit tests have passed now. @joemcgill @felixarntz, this is ready for re-review and commit as the initial part of the solution.

A commit was made that fixes the Trac ticket referenced in the description of this pull request.

SVN changeset: 59662
GitHub commit: 2f65488

This PR will be closed, but please confirm the accuracy of this and reopen if there is more work to be done.